-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Document platform-specific differences for std::process::exit
.
#38397
Document platform-specific differences for std::process::exit
.
#38397
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
/// function only considers the lower eight bits of `code`. For example, on | ||
/// these platforms, calling `exit` with values `0x00_00` and `0x0f_00` will be | ||
/// handled identically. Other platforms, like Windows, will take into account | ||
/// all 32 bits of `code`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to add a sentence at the end like:
For specific information about your platform, please check <insert resource here>
But I wasn't sure what resource to put there. man 3 exit
? "your systems 'exit' function"?
¯\_(ツ)_/¯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to like to cite the opengroup docs as they've always seemed "extra official". Maybe a link to that could be added here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like your opengroup link says exit
only takes into consideration the least significant eight bits. I have hesitations about just citing that as it doesn't match the behavior on Windows which takes into account all 32 bits with no truncation (as per this comment by @retep998).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry to be clear that opengroup page is a Unix-only thing, which is basically just explaining why only 8 bits are transmitted to the parent on Unix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Windows is the only tier 1~2 platform which I’m sure passes through all the 32 bits. Every other target may be passing through whatever number of bits they want, as long as its not less than 8 bits. I’m also not sure it is worth mentioning platforms which have obvious behaviour at all (i.e. Windows). Since tiers are an always-moving list, I’d perhaps not tie any explanation to that, but I still feel like it could be worded better. For example:
## Platform-specific behaviour
**Unix**: On UNIX-like (or POSIX, whichever you prefer) platforms it is unlikely that all the 32 bits of
status code will be visible to a parent process inspecting the exit code. On most UNIX-like platforms
only 8 least-significant bits are considered/evaluated/whatever is the best word here.
I do not think it is necessary to link to any external references here. I’m confident that mentioning this exceptional behaviour is necessary, but in case of references you’ll either end up having to reference documentation for every single supported platform (some of them have no documentation at all) or linking a POSIX spec, which is useless if you’re trying to figure out exact behaviour on your particular platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great feedback, thanks! I incorporated in your block of text you wrote above and made some minor tweaks.
Thanks @frewsxcv! In retrospect we should have probably made this argument a |
/// # Examples | ||
/// | ||
/// ``` | ||
/// use std::process; | ||
/// | ||
/// process::exit(0); | ||
/// ``` | ||
/// | ||
/// Due to [platform-specific behavior], the exit code for this example will be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great example.
111ff5c
to
0971cb1
Compare
@bors: r+ |
📌 Commit 0971cb1 has been approved by |
@bors rollup |
…exit, r=alexcrichton Document platform-specific differences for `std::process::exit`. Fixes rust-lang#35046.
/// Due to [platform-specific behavior], the exit code for this example will be | ||
/// `0` on Linux, but `256` on Windows: | ||
/// | ||
/// ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to fail on Windows, because it's a non-zero return code. So I'm going to mark as no_run
.
0971cb1
to
4d392d3
Compare
@bors r=alexcrichton |
📌 Commit 4d392d3 has been approved by |
…exit, r=alexcrichton Document platform-specific differences for `std::process::exit`. Fixes rust-lang#35046.
Fixes #35046.